-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pass tidal heating to Aragog and SPIDER #282
Conversation
src/proteus/interior/aragog.py
Outdated
# Update tidal heating inside Aragog | ||
tidal_value= 0.0 | ||
if config.interior.tidal_heat: | ||
if config.orbit.dummy: | ||
tidal_value = config.orbit.dummy.H_tide | ||
tidal_value /= aragog_solver.parameters.scalings.power_per_mass | ||
aragog_solver.parameters.energy.tidal_value = tidal_value | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These lines should go into the SetupAragogSolver
function (done once for all from the config).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have now moved this, although in the future we will also need to make sure it's updated in the UpdateAragogSolver function, since the tidal heat production will evolve over time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have now moved this, although in the future we will also need to make sure it's updated in the UpdateAragogSolver function, since the tidal heat production will evolve over time.
Make sense, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does the dummy test values change? The input file is the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I realised that input/dummy.toml
has not enabled tidal heating while tests/integration/dummy.toml
has. Why do we have two dummy input file by the way!
But now all the plots and data of the physical test have changed while the tidal heating has not been enabled in tests/integrtation/physical.toml
! What is happening here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having two separate dummy configuration files is quite confusing. I'll remove the one in the input folder.
The image files from the physical test have changed in the sense of binary files, I guess due to some numerical thing or as a result of #192. However, if you look at the before/after images with the GitHub comparison tool, you can see that the simulation results are identical.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK for the plots but what about the helpfile? If it is just tiny differences the test should pass anyway without having to push a new one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally, it seems to be a case that new test validation files have to be uploaded with each new code change. Emma had to that as well recently for the Zephyrus changes. That somehow defeats their purpose, sth to chat about in the meeting at 11.00.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lsoucasse good point. It is not clear to me at the moment why exactly the values seem to have changed in the csv file.
It seems to be a result of Aragog returning a different surface temperature, for some reason. This change is on the order of 1e-3 Kelvin, but it would be good to know why it exists at all
Passes tidal heating rate from PROTEUS to SPIDER and Aragog. Currently, this heating rate is just a constant value (W/kg) which is homogeneous throughout the mantle of the planet.
This can only be specified through the dummy orbit module (in the config file) for now.
Requires ExPlanetology/aragog#34 to be merged first.
Closes #281
Also includes patch for #280.